-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed Style hash #2346
Fixed Style hash #2346
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2346 +/- ##
==========================================
- Coverage 98.88% 98.71% -0.18%
==========================================
Files 73 73
Lines 7629 7761 +132
==========================================
+ Hits 7544 7661 +117
- Misses 85 100 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🙂
Fixes a performance bug in the Style class.
Essentially the hash calculation was inconsistent. Things that were equal would sometimes have different hashes. I noticed this when adding
lru_cache
toStyle.__add__
. What should have been an easy win caused dramatically reduced performance, because it broken the mechanics of storing Styles in a dict.The hashing is now in once place. It is also lazy, so the hash is stored the first time it is used.